Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a nightly/test workflow for flatpak and adapt manifest accordingly #523

Merged
merged 15 commits into from
Nov 13, 2024

Conversation

matterhorn103
Copy link
Contributor

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

@matterhorn103
Copy link
Contributor Author

Uses https://github.com/marketplace/actions/flatpak-builder

According to the link, it seems the artifact should be uploaded automatically without needing a specific upload step?

Keen for feedback, if we need a cleanup step etc. This is just based on the example given.

@@ -0,0 +1,72 @@
name: Build Flatpak

Check notice

Code scanning / Checkov (reported by Codacy)

Ensure top-level permissions are not set to write-all Note

Ensure top-level permissions are not set to write-all
@matterhorn103
Copy link
Contributor Author

matterhorn103 commented Nov 10, 2024

Ok well glu and rapidjson are building fine but now it's failing with:

 -- Installing: /app/share/doc/RapidJSON/html/classrapidjson_1_1_encoded_input_stream.html
Committing stage build-rapidjson to cache
========================================================================
Building module avogadro2 in /__w/avogadroapp/avogadroapp/.flatpak-builder/build/avogadro2-1
========================================================================
Error: module: avogadro2: Can't find CMakeLists.txt
Error: Build failed: Error: The process '/usr/bin/xvfb-run' failed with exit code 1

and I really don't know why. As far as I can tell openchemistry was successfully cloned into /__w/avogadroapp/avogadroapp i.e. the source root, and all the other repos were successfully cloned to their respective subdirectories.

@ghutchis
Copy link
Member

You might want to use this to debug the failure. I can't get the raw log.

   - name: Setup tmate session
      if: ${{ failure() }}
      uses: mxschmitt/action-tmate@v3

@matterhorn103
Copy link
Contributor Author

Does it fail or does it just time out?

@matterhorn103
Copy link
Contributor Author

In the annotations it says

System.IO.IOException: No space left on device : '/home/runner/runners/2.320.0/_diag/Worker_20241111-000701-utc.log'

If it ran out of available storage, that would explain why it just stops without giving an error


- name: Setup tmate session
if: failure()
uses: mxschmitt/action-tmate@v3

Check warning

Code scanning / Semgrep (reported by Codacy)

An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Warning

An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release.
@matterhorn103
Copy link
Contributor Author

matterhorn103 commented Nov 11, 2024

I've turned off the .flatpak-builder cache to see if that helps, added tmate at the same time.

I noticed that build_cmake.yml includes a cleanup step, as does build_m1.yml, but build_qt6.yml doesn't. How come the discrepancy? Is the cleanup step preferred, and if so should I update all the workflows to match?

@matterhorn103
Copy link
Contributor Author

Ok well now there seem to be problems downloading the third-party dependencies (I get the same error when trying to build the flatpak locally) so I guess I'll just leave this for a few days and come back to it...

Signed-off-by: Matthew J. Milner <[email protected]>
@matterhorn103
Copy link
Contributor Author

matterhorn103 commented Nov 12, 2024

Ah, I didn't see that the versions of spglib and libarchive looked for were bumped in the main repo... Looks like it'll work now.

Adding GitHub Actions to the openchemistry repo as planned will help avoid a mismatch.

@ghutchis
Copy link
Member

In particular, the spglib bump fixed cmake build problems on Windows

@matterhorn103
Copy link
Contributor Author

matterhorn103 commented Nov 12, 2024

In its current state this now builds fine locally, so I don't think the manifest is the issue. I don't know what the problem is really. It just gets randomly stuck during the open babel build step.

@ghutchis
Copy link
Member

That's where I'd definitely use the remote shell. Does Flatpak build in a Docker image or something? If so, maybe it's running out of space?

@matterhorn103
Copy link
Contributor Author

matterhorn103 commented Nov 12, 2024

Ah yes, that'll be it. I just used the workflow suggested by the Action's author, and that indeed runs it in a container. I'll increase the resources allocated to it.

I did think it seemed like a low storage issue but I checked my local build and everything only comes to a few GB, which is way less than the runners have, so I ruled it out on that basis. Hadn't thought about containers being potentially limited even further.

@matterhorn103
Copy link
Contributor Author

Ok it's now set up to build on the runner directly but it now can't do the clone like it's meant to be able to :(

Don't know if running flatpak-builder as sudo would fix that but seems like a bad idea right?

@ghutchis
Copy link
Member

Don't know if running flatpak-builder as sudo would fix that but seems like a bad idea right?

The runners are images, so you can sudo if needed. If you look at the other build actions, they all install a variety of packages.

@matterhorn103
Copy link
Contributor Author

In the last workflow run the actual build was successful, even if it did take an hour, so with 2c56246 I'm expecting it to pass and this should now be ready to merge @ghutchis :)

@ghutchis
Copy link
Member

Okay, I'l take a look tomorrow morning Pittsburgh time.

@ghutchis ghutchis merged commit 6694257 into OpenChemistry:master Nov 13, 2024
28 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants